Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dnf5 on Fedora 40+ #1332

Merged
merged 1 commit into from
Feb 16, 2024
Merged

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented Feb 16, 2024

@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

Alternatively, this can be hardcoded in all the fedora-40 configs. And keep doping that with all new ones.

Or, we can let it be dnf5 and hardcode it to dnf in all the old ones.

@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

working on changelog entry

@praiskup
Copy link
Member

Hm, actually ... have you tried it works?

@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

I only tried rendering such a template in Python. Will test to ensure.

@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

ERROR: Config error: /etc/mock/fedora-39-x86_64.cfg: invalid syntax (, line 15)

Copy link
Member

@praiskup praiskup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you

@hroncok hroncok marked this pull request as ready for review February 16, 2024 14:12
@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

Tested with F40 and F39 and it works as expected.

@hroncok
Copy link
Contributor Author

hroncok commented Feb 16, 2024

For the record, the first attempt created an invalid config and no CI job seems to have noticed that. Perhaps a test should be added that the configs are at least parse-able?

@praiskup
Copy link
Member

@hroncok I can not find the old testing-farm run now :-/ but it already appeared failed/red actually.
In this case, the tests failed because we do most of the testing on Fedora 39 host against the Fedora 38 (branched) configs (current status quo).

But in general, we don't let the test for configs fail the CI. Some of the configs are known to fail; you have to take a look manually into the logs to see the failures.

@praiskup
Copy link
Member

Ad tests, take a look at the failure here.

@praiskup
Copy link
Member

I can see Fedora-39:x86_64:/testing-farm/plans/behave | COMPLETE | OK | PASSED now so the basic tests already passed. Merging earlier and releasing so we have it in updates-testing ASAP.

@praiskup praiskup merged commit 9559824 into rpm-software-management:main Feb 16, 2024
19 of 20 checks passed
@hroncok hroncok deleted the f40-dnf5 branch February 16, 2024 14:41
praiskup added a commit to praiskup/copr that referenced this pull request Feb 20, 2024
The Jinja statements are only expanded in the values of the config_opts
dictionary, not in the keys. The new mock-core-configs version 40.2
contains a Jinja condition in the package_manager value. Therefore, we
cannot simply use it as a key in config_opts.

Since yum.conf and dnf.conf are aliases (starting from Mock 2.1), and
dnf5.conf (starting from Mock 5.0+), we can avoid using the variable as
a key entirely.

Relates: rpm-software-management/mock@5d620ad326d
Relates: rpm-software-management/mock#1332
praiskup added a commit to fedora-copr/copr that referenced this pull request Feb 21, 2024
The Jinja statements are only expanded in the values of the config_opts
dictionary, not in the keys. The new mock-core-configs version 40.2
contains a Jinja condition in the package_manager value. Therefore, we
cannot simply use it as a key in config_opts.

Since yum.conf and dnf.conf are aliases (starting from Mock 2.1), and
dnf5.conf (starting from Mock 5.0+), we can avoid using the variable as
a key entirely.

Relates: rpm-software-management/mock@5d620ad326d
Relates: rpm-software-management/mock#1332
@hroncok
Copy link
Contributor Author

hroncok commented Apr 11, 2024

This broke my own CI strcipt. We use config_opts[f"{config_opts.package_manager}.conf"] += ... which now fails with:

ERROR: Config error: /tmp/fedora-39-x86_64-ci.cfg: '{% if releasever|int >= 40 %}dnf5{% else %}dnf{% endif %}.conf'

Is this something we should try to fix here, or was this never actually supported?

@xsuchy
Copy link
Member

xsuchy commented Apr 11, 2024

This is evaluated by https://pypi.org/project/templated-dictionary/ so it should work.

Can you open a new issue and put there your whole config so we can reproduce it?

@hroncok
Copy link
Contributor Author

hroncok commented Apr 11, 2024

#1357

@praiskup
Copy link
Member

Is this something we should try to fix here, or was this never actually supported?

We e.g. did this in Copr: fedora-copr/copr#3152

@hroncok
Copy link
Contributor Author

hroncok commented Apr 11, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants